Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates #143

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Updates #143

wants to merge 12 commits into from

Conversation

JoshData
Copy link
Member

Hi all. I'm working on an update to https://uslaw.link/. Here are some updates to citation. The first few commits in this PR are just about updating packages with security issues. The last few add some minor new functionality.

Ran 'npm audit fix --force' and then fixed the gulpfile.
* Update GPO (govinfo) and uscode.house.gov links.
* Remove 'beta' from code.dccouncil.us links.
* Remove legislink.org linker because the site is being discontinued.
* Disable the LoC links for Statutes at Large volumes because it's not a deep link.
* Other tweaks.
Copy link
Contributor

@divergentdave divergentdave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good, thanks for working on this! (I haven't kicked the tires on it, as I'm writing this review from a bus) I think we might benefit from some tests to compare the output of fromId to the existing find method. The test could find a citation from a fixed string, take the id from that object, pass it to fromId, and then do a deep comparison of the two objects. They should be identical, save for the fields relating to the text searched by find. Thanks again!

links/libraryofcongress.js Show resolved Hide resolved
@JoshData
Copy link
Member Author

JoshData commented Apr 4, 2020

Tests added! :)

@JoshData JoshData force-pushed the updates branch 4 times, most recently from 6a5922e to f020af9 Compare June 7, 2020 13:03
@JoshData
Copy link
Member Author

JoshData commented Jun 7, 2020

It looks like nodeunit is permanently broken --- I can't get it to install on Travis, so the build is failing.

But it works fine locally and tests are passing. Everyone OK to merge?

@divergentdave
Copy link
Contributor

I think the build is failing because .travis.yml specifies a very old version of node, and ejs's postinstall script is too new for it.

@divergentdave
Copy link
Contributor

Oh dear, it's using v0.10 now, the default is slightly older!

The ejs package required by nodeunit failed to install on this old version. .travis.yml is revised to use the latest stable node version per https://docs.travis-ci.com/user/languages/javascript-with-nodejs/.
@JoshData
Copy link
Member Author

JoshData commented Jun 7, 2020

Nice catch! Ok finally got it working.

Copy link

@Heydude13 Heydude13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uploading Content Restrictions-2015_H1(1).csv.pdf …

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants